Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FINAL] feat: Allow accepting and burning cycles in replicated queries #3760

Merged
merged 4 commits into from
Jan 23, 2025

Conversation

mraszyk
Copy link
Contributor

@mraszyk mraszyk commented Nov 15, 2024

Given a previous change that introduced formally the concepts of replicated and non-replicated queries, we can now extend the spec to allow accepting and burning cycles in replicated queries. This allows for creating reasonable business models for canisters by requiring callers in replicated mode to always make payments for accessing their endpoints.

@mraszyk mraszyk added the interface-spec Changes to the IC Interface Specification label Nov 18, 2024
github-merge-queue bot pushed a commit to dfinity/ic that referenced this pull request Jan 22, 2025
This PR allows canisters to accept and burn cycles when executing
queries in replicated mode (e.g. as an ingress message or when another
canister calls the query method). See also spec
[PR](dfinity/portal#3760).

This allows canisters to expect some payment in cases someone is calling
an expensive endpoint similar to how this is possible in update calls.
Given that replicated queries run across all nodes, there's no technical
issue in persisting cycles changes and it gives developers another way
of protecting expensive endpoints of their canisters.

The main parts of the change are the following:

Previously the sandbox would return an optional `StateModifications`
object as changes would need to be persisted in update calls but not in
queries. Since we would like to persist cycles changes which are part of
the system state of the canister, the struct is modified to have an
`Option<ExecutionStateModifications>` to capture the optionality of
applying execution state changes while it always includes
`SystemStateChanges`. The system API is also adjusted to return only the
changes that are relevant per context of execution.

The benefit of this is that it makes more clear what parts of the
canister state can be persisted. It also allows to handle more uniformly
other parts of the system state that need to be persisted for replicated
queries, like canister logs. Instead of handling them separately, they
could now be simply included when applying changes to the system state
(not included in this PR but would be a possible follow up). Further
future changes that have similar characteristics (e.g. persisting
canister metrics in a way similar to logs) could be incorporated more
easily.

The second big chunk of changes is in the `replicated_query` execution
handler. The handler is modified to allow for handling the acceptance of
cycles and refunding any remaining amount to the caller.

Some tests were also added to confirm things work as expected.
@mraszyk
Copy link
Contributor Author

mraszyk commented Jan 22, 2025

Merged into replica: dfinity/ic@178acea

@mraszyk mraszyk enabled auto-merge (squash) January 23, 2025 15:42
@mraszyk mraszyk merged commit 14cd3df into master Jan 23, 2025
7 checks passed
@mraszyk mraszyk deleted the dimitris/accept-cycles-replicated-queries branch January 23, 2025 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interface-spec Changes to the IC Interface Specification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants